Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[COMMON] Replace NodeInfo with Broker #1763

Merged
merged 15 commits into from
May 24, 2023

Conversation

chaohengstudent
Copy link
Collaborator

related to #1721

主要改動

NodeInfo 移除,改由 Broker 物件取代

NodeInfo.of(int id, String host, int port) -> Broker.of(int id, String host, int port)
NodeInfo.of(org.apache.kafka.common.Node node) -> Broker.of(org.apache.kafka.common.Node node)

這兩個建構方法會導致其他資訊為空值(目前 Admin.partitions 和 Admin.replicas 中使用到的 Broker ),不知道是否合理還是需要確保資訊的完整

修改 Broker

  • 改為 record

相關改動

Admin.nodeInfos() -> Admin.brokers()
Replica.nodeInfo() -> Replica.broker()

TODO

若此 pr 可以合併將在 #1721 完成下述:
修改 toBytes(ClusterInfo value)
修改 readClusterInfo(byte[] bytes)
因序列化格式須修改,測試先暫時移除

# Conflicts:
#	app/src/main/java/org/astraea/app/backup/Backup.java
#	app/src/main/java/org/astraea/app/web/ReassignmentHandler.java
#	app/src/main/java/org/astraea/app/web/SkewedPartitionScenario.java
#	common/src/test/java/org/astraea/common/cost/ReplicaLeaderCostTest.java
#	common/src/test/java/org/astraea/common/partitioner/StrictCostPartitionerPerfTest.java
@chia7712
Copy link
Contributor

Broker.of(org.apache.kafka.common.Node node)

請問一下這個方法的用途?如果是從Admin來建立的話應該是可以建立完整的資訊?

# Conflicts:
#	common/src/main/java/org/astraea/common/cost/utils/ClusterInfoSensor.java
@chaohengstudent
Copy link
Collaborator Author

請問一下這個方法的用途?如果是從Admin來建立的話應該是可以建立完整的資訊?

目前情境是當 Broker 離線時會沒有辦法拿到其他資訊

# Conflicts:
#	app/src/test/java/org/astraea/app/web/BalancerHandlerTest.java
@chia7712
Copy link
Contributor

麻煩修正一下衝突,以及標題名稱修改一下

@chia7712
Copy link
Contributor

標題名稱要加讓 [COMMON]

@chaohengstudent chaohengstudent changed the title Replace NodeInfo with Broker [COMMON] Replace NodeInfo with Broker May 22, 2023
Copy link
Collaborator

@garyparrot garyparrot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@garyparrot garyparrot merged commit ced840f into opensource4you:main May 24, 2023
@@ -186,12 +186,13 @@ public static byte[] toBytes(BeanObject value) {
return beanBuilder.build().toByteArray();
}

// TODO: Due to the change of NodeInfo to Broker. This and the test should be updated.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

所以這個有後續追蹤的議題 or PR 嗎??

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#1721 嗎?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

是的

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants